Skip to content

Resolve and build dependency wheels in one PR with the dependency bump that triggers the build#23063

Open
iliakur wants to merge 17 commits intomasterfrom
ik/AI-6182/pass-package-base-url
Open

Resolve and build dependency wheels in one PR with the dependency bump that triggers the build#23063
iliakur wants to merge 17 commits intomasterfrom
ik/AI-6182/pass-package-base-url

Conversation

@iliakur
Copy link
Copy Markdown
Contributor

@iliakur iliakur commented Mar 26, 2026

What does this PR do?

  • Pass INTEGRATIONS_WHEELS_STORAGE to triggered agent builds
  • Update ddev size tools to handle both lockfile URL formats
  • Upload wheels to dev/ prefix and use ${INTEGRATIONS_WHEELS_STORAGE} in lockfiles
  • Update promote.py to parse ${INTEGRATIONS_WHEELS_STORAGE} lockfile format
  • Change publish job to run on PRs only and commit lockfiles to branch
  • Add promote-gate and promote-wheels workflows
  • Update upload tests and add promote tests for new lockfile format

Motivation

We're fed up with having to make 2 PRs every time we resolve dependencies!

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

iliakur and others added 7 commits March 26, 2026 15:25
When integrations-core triggers agent CI builds, pass PACKAGE_BASE_URL
pointing to dev storage. This prepares for lockfiles switching to
${PACKAGE_BASE_URL}/... format so PR-triggered builds use dev wheels.
No-op today since current lockfiles use hardcoded URLs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Support both the legacy hardcoded URL format and the new
\${PACKAGE_BASE_URL}/... template format in lockfile entries.
Resolves \${PACKAGE_BASE_URL} to the stable base URL before
downloading wheels for size calculations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wheels are now uploaded to dev/{artifact_type}/{project_name}/ paths
in GCS instead of the unprefixed paths. Lockfile entries are templated
with \${PACKAGE_BASE_URL} so pip resolves the URL at install time using
either the dev or stable base URL depending on the environment.

Also fix brittle index extraction in generate_artifact_listings and
list_wheels_with_prefix to use split('/')[-1] and split('/')[-2]
instead of hardcoded indices.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lockfiles now use \${PACKAGE_BASE_URL}/... template entries instead of
hardcoded URLs. Update url_to_blob_path to extract the relative path
from \${PACKAGE_BASE_URL}/... entries, then prepend dev/ when looking up
blobs in GCS and stable/ for the promotion destination.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of creating a separate PR with updated lockfiles, the publish
job now commits them directly to the PR branch. This collapses the
two-PR dependency update workflow into a single PR.

- Trigger: pull_request only (remove push and workflow_dispatch)
- Permission: contents: write (needed for git push)
- Token: GitHub App token checked out before checkout so push works
- Replace peter-evans/create-pull-request with a git commit + push step

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
promote-gate.yaml runs on every PR push to master/7.*.*. If dependency
files (agent_requirements.in or .deps/resolved/) changed, it sets the
promote-wheels commit status to pending, blocking merge. Otherwise it
sets it to success (no promotion needed).

promote-wheels.yaml is triggered via workflow_dispatch (by ddev promote).
It checks out the PR branch at the given SHA, runs .builders/promote.py
to copy wheels from dev/ to stable/ in GCS, then sets the promote-wheels
commit status to success and posts a comment on the PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
test_upload.py: update all blob path assertions to use dev/ prefix
and all lockfile URL assertions to use \${PACKAGE_BASE_URL} format.
Update generate_artifact_listings assertions to use dev/-prefixed paths.

test_promote.py (new): test lockfile parsing, url_to_blob_path,
collect_relative_paths, GCS copy with correct dev/stable paths,
idempotency, and failure on missing source blobs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

⚠️ Recommendation: Add qa/skip-qa label

This PR does not modify any files shipped with the agent.

To help streamline the release process, please consider adding the qa/skip-qa label if these changes do not require QA testing.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14bd37bd66

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 54.54545% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (5fa36f1) to head (d33448c).
⚠️ Report is 33 commits behind head on master.

Additional details and impacted files
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aiuto
aiuto previously approved these changes Mar 31, 2026
iliakur and others added 5 commits April 2, 2026 10:48
PACKAGE_BASE_URL was a full URL env var, which is more than needed and
potentially dangerous. Replace it with INTEGRATIONS_WHEELS_STORAGE whose
value is only "dev" or "stable".

Lockfile entries now use the form:
  https://agent-int-packages.datadoghq.com/\${INTEGRATIONS_WHEELS_STORAGE}/...

The base domain is hardcoded; only the storage tier is variable. This
limits what a compromised env var could redirect to.

Update all affected files: upload.py, promote.py, size tools, tests,
build_agent.yaml, and the promotion workflows.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rename promote-gate.yaml -> dependency-wheel-promotion-gate.yaml and
promote-wheels.yaml -> dependency-wheel-promotion.yaml so the two
related workflows sort next to each other and their purpose is explicit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The actions/checkout shallow clone does not include the base branch,
so git diff --name-only against origin/<base> fails. Replace the git
command with a GitHub API call (pulls.listFiles) which does not require
a checkout at all.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fork PRs cannot access repo secrets (GCS credentials, GitHub App key)
or the Workload Identity Provider used by google-github-actions/auth.
Add !github.event.pull_request.head.repo.fork to the publish job condition
so it only runs on PRs from branches within the repo.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ddev dep promote <PR_URL> extracts the PR number from the URL, fetches
the head SHA and branch from the GitHub API, then dispatches the
dependency-wheel-promotion workflow via workflow_dispatch.

This avoids both wasted runner minutes (no issue_comment polling) and
new infrastructure (no webhook handler). The workflow only runs when
explicitly dispatched.

Also add get_pr_head() and dispatch_workflow() to GitHubManager.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@temporal-github-worker-1 temporal-github-worker-1 bot dismissed aiuto’s stale review April 2, 2026 08:49

Review from aiuto is dismissed. Related teams and files:

  • agent-build
    • .builders/promote.py
    • .builders/tests/test_promote.py
    • .builders/tests/test_upload.py
    • .builders/upload.py
@iliakur
Copy link
Copy Markdown
Contributor Author

iliakur commented Apr 2, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd1d84f3f0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +79 to +81
if not rel_paths:
print("No templated wheels found in lockfiles — nothing to promote.")
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fail promotion when lockfiles yield zero wheel paths

Returning success when rel_paths is empty can falsely mark promotion as complete even when nothing was copied to stable storage. In this commit, resolved lockfiles use ${PACKAGE_BASE_URL}/... entries, while url_to_blob_path only recognizes ${INTEGRATIONS_WHEELS_STORAGE}; this drops every wheel path, hits this early return, and lets the workflow set dependency-wheel-promotion to success without promoting any artifacts.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@sarah-witt sarah-witt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great!! Very exciting 🎉

I have a few non-blocking comments:

  • lazy-loader moved from built to external, do you know why?
  • Could you/have you tested the new lockfiles with the agent changes?
  • In the future, could we have validation for lockfiles and deps being in sync?
  • Do we know how this will affect the size calculations/will the current approach still work with this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants